-
Notifications
You must be signed in to change notification settings - Fork 99
gsoc containing json serialization handler and dubbo codec #51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Pull Request SummaryThis pull request includes the complete implementation based on previous discussions and feedback. It also incorporates suggestions from @cnzakii, who has been mentoring and guiding the direction. 🔑 Key Highlights
self.users_db = [
User(
id=1,
name="Alice",
email="[email protected]",
age=30,
login_history=[
LoginRecord(timestamp=datetime.utcnow() - timedelta(days=1), ip_address="192.168.1.10"),
LoginRecord(timestamp=datetime.utcnow(), ip_address="192.168.1.11")
],
meta=UserMeta(tags=frozenset(["admin", "beta"]), scores=(88, 92, 85))
),
User(
id=2,
name="Bob",
email="[email protected]",
age=25,
login_history=[
LoginRecord(timestamp=datetime.utcnow() - timedelta(hours=2), ip_address="192.168.1.20")
],
meta=UserMeta(tags=frozenset(["tester"]), scores=(75, 80, 70))
)
] |
@cnzakii check this out |
@cnzakii in this repo i have a code from which i run the server and client |
src/dubbo/classes.py
Outdated
|
||
from typing import Any, Callable, Optional, Union,Type | ||
from abc import ABC, abstractmethod | ||
from pydantic import BaseModel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This place will still cause strong dependency on pydantic
arg_serialization=(request_serializer, None), | ||
return_serialization=(None, response_deserializer), | ||
rpc_type=RpcTypes.CLIENT_STREAM.value, | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also hope to see the implementation on the server side
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cnzakii
If you don't mind can you explain me about this
like most of the work of server can be handle by the rpc handler
or
Am i missing something ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can review the proposal I sent in Slack, which also contains the corresponding Server interface design. You should also implement the construction method descriptors on the Server side, select the serialization method, and so on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
Now i understand
Thx for clarification
@cnzakii |
@aditya0yadav While it lacks some generality, the overall work is good. Please make optimizations based on my comments, complete the remaining tasks, and submit the relevant code. |
@cnzakii ok sir |
@cnzakii, then there's no need for an RPC method handler, right? |
@cnzakii |
@cnzakii Please check out the pull request , sir |
Okay, I'll do a code review as soon as I can. |
Quick View: missing Test, please supplement. |
Hi sir I wanted to share a quick update. This project still requires Hessian integration, and until that is added, there is more work to be done. I plan to continue working on this project after GSoC, focusing on Hessian integration and fixing additional bugs in the system. Thank you, |
@cnzakii |
…e handled by the pyhessian
@cnzakii Please take a look at this |
@cnzakii done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please carefully review my comments and either address the issues I raised or clearly respond in the comments explaining why no changes are made.
@cnzakii Please review this. I refactored the JSON and Protobuf code into multiple modules, using the Extension Loader to manage and call the appropriate handlers. JSON: Multiple type handlers are now supported via the Extension Loader. The system will use ujson or orjson depending on availability and performance. Protobuf: If the user is using BetterProto or Google Protoc, the correct message handler will be automatically inferred and used. This is also managed through the Extension Loader, so no manual intervention is needed. |
@cnzakii Please check it out |
@cnzakii
Sir please check out all the changes
…On Thu, 4 Sep, 2025, 19:02 Zaki, ***@***.***> wrote:
***@***.**** commented on this pull request.
Please carefully review my comments and either address the issues I raised
or clearly respond in the comments explaining why no changes are made.
------------------------------
In samples/llm/chat_pb2.py
<#51 (comment)>:
> @@ -1,13 +1,15 @@
-# -*- coding: utf-8 -*-
# Generated by the protocol buffer compiler. DO NOT EDIT!
As the hint says, DO NOT EDIT!
------------------------------
In src/dubbo/classes.py
<#51 (comment)>:
> + def __init__(self, model_type: Optional[type[Any]] = None, **kwargs):
+ self.model_type = model_type
+
+ @AbstractMethod
+ def encode(self, data: Any) -> bytes:
+ pass
+
+ @AbstractMethod
+ def decode(self, data: bytes) -> Any:
+ pass
+
+
+class CodecHelper:
+ @staticmethod
+ def get_class():
+ return Codec
Why is an additional CodecHelper wrapper needed?
------------------------------
In src/dubbo/client.py
<#51 (comment)>:
> request_serializer: Optional[SerializingFunction] = None,
response_deserializer: Optional[DeserializingFunction] = None,
) -> RpcCallable:
- return self._callable(
- MethodDescriptor(
- method_name=method_name,
- arg_serialization=(request_serializer, None),
- return_serialization=(None, response_deserializer),
- rpc_type=RpcTypes.UNARY.value,
+ """
+ Create RPC callable with the specified type.
+ """
+ print("2", params_types)
What is this?
------------------------------
In src/dubbo/client.py
<#51 (comment)>:
> # Determine serializers
if request_serializer and response_deserializer:
req_ser = request_serializer
res_deser = response_deserializer
else:
- req_ser, res_deser = DubboTransportService.create_serialization_functions(
- codec or "json", # fallback to json
- parameter_types=p_types,
- return_type=r_type,
+ req_ser, res_deser = DubboSerializationService.create_serialization_functions(
+ codec or "json",
JSON cannot simply be used as a fallback strategy. Unless you have a
complete decision mechanism, do not set it as a fallback (I already
mentioned this in the previous review...).
------------------------------
In src/dubbo/codec/json_codec/json_transport_base.py
<#51 (comment)>:
> + self.type_handlers[obj_type] = handler
+
+ def register_plugin(self, plugin: TypeHandlerPlugin) -> None:
+ """Register a plugin"""
+ self.plugins.append(plugin)
+
+ def get_handler(self, obj: Any) -> Optional[Callable[..., Any]]:
+ """Get handler for object - check dict first, then plugins"""
+ obj_type = type(obj)
+ if obj_type in self.type_handlers:
+ return self.type_handlers[obj_type]
+
+ for plugin in self.plugins:
+ if plugin.can_serialize_type(obj, obj_type):
+ return plugin.serialize_to_dict
+ return None
Same old issue... Please integrate with Dubbo’s extension...
------------------------------
In src/dubbo/codec/json_codec/json_transport_codec.py
<#51 (comment)>:
> + elif isinstance(obj, date):
+ return {"__date__": obj.isoformat()}
+ elif isinstance(obj, time):
+ return {"__time__": obj.isoformat()}
+ elif isinstance(obj, Decimal):
+ return {"__decimal__": str(obj)}
+ elif isinstance(obj, set):
+ return {"__set__": list(obj)}
+ elif isinstance(obj, frozenset):
+ return {"__frozenset__": list(obj)}
+ elif isinstance(obj, UUID):
+ return {"__uuid__": str(obj)}
+ elif isinstance(obj, Path):
+ return {"__path__": str(obj)}
+ return {"__fallback__": str(obj), "__type__": type(obj).__name__}
+
Please refer to the implementation in compression, which is the simplest
extension mechanism, including both interface definition and implementation.
------------------------------
In src/dubbo/proxy/handlers.py
<#51 (comment)>:
> return self._method_descriptor
+ @staticmethod
+ def get_codec(**kwargs) -> tuple:
+ return DubboTransportService.create_serialization_functions(**kwargs)
+
+ @classmethod
+ def _infer_types_from_method(cls, method: Callable) -> tuple:
+ try:
+ type_hints = get_type_hints(method)
+ sig = inspect.signature(method)
+ method_name = method.__name__
+ params = list(sig.parameters.values())
+ if params and params[0].name == "self":
This issue still hasn’t been resolved.
------------------------------
In src/dubbo/codec/protobuf_codec/protobuf_codec_handler.py
<#51 (comment)>:
> +# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from typing import Any, Type, Protocol, Optional
+from abc import ABC, abstractmethod
+import json
+from dataclasses import dataclass
+
+# Betterproto imports
This issue still hasn’t been resolved.
—
Reply to this email directly, view it on GitHub
<#51 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BHWNCTKQUGUVUHHMATJOVST3RA5NHAVCNFSM6AAAAACAYF7C2SVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTCOBVGIYDSNRWG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
What is the purpose of the change
ISSUE: #issue_number
Brief changelog
Verifying this change
Checklist